Update the makefile to regenerate makedependfile when the developer changes the content of makedependfile.SH #23402

dlaugt · 2025-07-04T13:39:04Z

The perl5 directory contains *.SH files that generate underlying files. Configure calls those *.SH at the beginning to create the underlying files. Once Configure is done, there is a makefile that handles the compilation in an incremental way. So, during the development, if the developer changes one of those *.SH files and runs make, the makefile will regenerate the underlying file and eventually will recompile what needs to be recompiled according to this change.

This is true for all *.SH except for makedepend_file.SH. If the developer changes makedepend_file.SH and runs make then nothing happens.

The goal of this pull request is to handle the incremental compilation when a developer changes makedepend_file.SH.

Before the pull request:
touch makedepend_file.SH && make
=> Nothing happens

After the pull request:
touch makedependfile.SH && make
=> makedependfile is regenerated
=> *.depends are regenerated
=> makefile is regenerated with dependencies

Adding makedependfile.SH in variable SH of Makefile.SH creates the target makedependfile in the generated makefile:

diff before/perl5/Makefile after/perl5/Makefile
785a786,788
> makedependfile: makedependfile.SH config.sh
>       $(SHELL) makedependfile.SH
>

An extra consequence is that makedependfile is deleted with the rest of generated files by rm -rf $(addedbyconf). We don't need to have a specific -rm makedepend_file in the makefile.

The renaming of makedepend_file.SH to makedependfile.SH has been done to work fine with SH_to_target(). This function is used to build the name of the makefile target with the script name by replacing the underscore by dot. Here is the code of SH_to_target() where there is a sed of _ by .:

SH_to_target() {
    echo $@ | sed -e s/\\\.SH//g -e s/_/./g
}

As the underlying file is a script, it was more coherent to name the target / underlying file as makedependfile instead of makedepend.file. This to have a similar naming convention as the other generated scripts: cflags, makedepend, myconfig, runtests.

PS: The description of this pull request has been rewritten after the remarks of the participants of this conversation.

This set of changes does not require a perldelta entry.

richardleach · 2025-07-04T15:04:22Z

The generated file has been renamed from makedepend_file to makedepend.file to be in line with SH_to_target() of Makefile.SH. SH_to_target() replaces in the script name the underscore by dot.

Please could you explain what making this change accomplishes?

Whether the resulting file is named makedepend_file or makedepend.file doesn't seem to make any difference.

dlaugt · 2025-07-04T15:23:13Z

Please could you explain what making this change accomplishes?
Whether the resulting file is named makedepend_file or makedepend.file doesn't seem to make any difference.

Exactly, makedepend_file or makedepend.file doesn't make any difference. However, I've renamed the generated file because SH_to_target() with makedepend_file.SH generates the target makedepend.file instead of makedepend_file as follow:

makedepend.file: makedepend_file.SH config.sh
    $(SHELL) makedepend_file.SH

Here is the code of SH_to_target() where there is a sed of _ by .:

SH_to_target() {
    echo $@ | sed -e s/\\\.SH//g -e s/_/./g
}

An example is the target config.h that is already generated by SH_to_target() with config_h.SH:

config.h: config_h.SH config.sh
    $(SHELL) config_h.SH

richardleach · 2025-07-04T16:17:43Z

However, I've renamed the generated file because SH_to_target() with makedepend_file.SH generates the target makedepend.file instead of makedepend_file

Yes, but why?

config.h is a C header file, so generating this filename follows the normal naming convention.

makedepend_file is a shell script and calling it makedepend.file instead doesn't follow the same sort of naming convention and arguably makes it less obvious that it's an executable file. It might make more sense to make the source file makedependfile_sh.SH to create makedependfile.sh, but again, what's the actual benefit in doing so?

dlaugt · 2025-07-04T16:47:23Z

Yes, but why?

I wanted to make as few changes as possible without changing the name of makedepend_file.SH committed in git. However, your observation is pertinent. Perhaps, we can rename the script makedepend_file.SH to makedependfile.SH to generate the script makedependfile which could have similar naming convention as the other generated scripts: cflags, makedepend, myconfig, runtests.

Anyway, the main goal of this pull request is not about renaming. It's about recreating automatically the generated script makedepend.file when the script makedepend_file.SH has been changed by the user. Like what we have for the other *.SH.

richardleach · 2025-07-04T16:59:07Z

Anyway, the main goal of this pull request is not about renaming. It's about recreating automatically the generated script makedepend.file when the script makedepend_file.SH has been changed by the user. Like what we have for the other *.SH.

Apologies if the answer is obvious, but why not add this target:

makedepend_file: makedepend_file.SH config.sh
    $(SHELL) makedepend_file.SH

dlaugt · 2025-07-04T17:07:04Z

This is also another way to do it with even less changes. However, I prefer to use the variable SH which is designed specifically for this purpose (eg, generating automically the files when *.SH has been changed by the user).

Leont · 2025-07-04T20:54:54Z

Perhaps, we can rename the script makedepend_file.SH to makedependfile.SH to generate the script makedependfile which could have similar naming convention as the other generated scripts: cflags, makedepend, myconfig, runtests.

Yeah I think that makes more sense than renaming it to makedepend.file.

dlaugt · 2025-07-05T11:03:59Z

Yeah I think that makes more sense than renaming it to makedepend.file.

Ok, I have added a commit for this renaming.

jkeenan · 2025-07-05T22:51:55Z

I ran sh ./Configure -des -Dusedevel && make test in blead and then after applying this p.r.

Before

-rwxrwxr-x 1 jkeenan jkeenan 4653 Jun  5 18:09 ./makedepend.SH
-rw-rw-r-- 1 jkeenan jkeenan 6499 Jul  4 13:56 ./makedepend_file.SH
-rwxrwxr-x 1 jkeenan jkeenan 3888 Jul  5 18:40 ./makedepend
-rwxrwxr-x 1 jkeenan jkeenan 5704 Jul  5 18:40 ./makedepend_file

After

-rwxrwxr-x 1 jkeenan jkeenan 4653 Jun  5 18:09 ./makedepend.SH
-rw-rw-r-- 1 jkeenan jkeenan 6492 Jul  5 18:42 ./makedependfile.SH
-rwxrwxr-x 1 jkeenan jkeenan 3888 Jul  5 18:43 ./makedepend
-rwxrwxr-x 1 jkeenan jkeenan 5703 Jul  5 18:43 ./makedependfile

Am I correct in thinking that the net effect of this p.r. is:

to change the name of source code file makedepend_file.SH to makedependfile.SH; and
to change the name of generated file makedepend_file to makedependfile?

Leont · 2025-07-06T14:05:27Z

Am I correct in thinking that the net effect of this p.r. is:

to change the name of source code file makedepend_file.SH to makedependfile.SH; and

to change the name of generated file makedepend_file to makedependfile?

Those are side-effects. The main effect is that «make makedependfile» is now a thing, which is relevant because that is a dependency of $(FIRSTMAKEFILE) (usually makefile).

Your question does point out that, this PR doesn't explain well why it does what it does. It needs a much better commit message IMO.

dlaugt · 2025-07-06T15:58:49Z

Am I correct in thinking that the net effect of this p.r. is:

to change the name of source code file makedepend_file.SH to makedependfile.SH; and

to change the name of generated file makedepend_file to makedependfile?

The main effect is makedependfile is generated automatically after the user changes the content of makedependfile.SH.

Once you ran sh ./Configure -des -Dusedevel && make test then simulate a user change on makedependfile.SH and run again the compilation by doing: touch makedependfile.SH && make

Before
Nothing happens

After
makedependfile is regenerated
*.c.depends are regenerated
makefile is regenerated with dependencies

This main effect is due to the fact that we have added makedependfile.SH in the variable SH of Makefile.SH. This adds the target makedependfile in generated makefile:

diff before/perl5/Makefile after/perl5/Makefile
785a786,788
> makedependfile: makedependfile.SH config.sh
>       $(SHELL) makedependfile.SH
>

dlaugt · 2025-07-06T16:46:20Z

Your question does point out that, this PR doesn't explain well why it does what it does. It needs a much better commit message IMO.

Ok, I've merged the two commits in one and have updated the commit message.

khwilliamson

This now makes sense to me. I'm not really qualified to know if this is a good solution or not, but I think it solves a real problem. I think that a comment should be added in the Makefile target dependency as to why. And I think the P.R. description should be updated to include the better description of the motivation behind this p.r.

dlaugt · 2025-07-06T20:36:05Z

I think the P.R. description should be updated to include the better description of the motivation behind this p.r.

I have changed the title and the description of the pull request. Hoping it is clearer...

I think that a comment should be added in the Makefile target dependency as to why.

Not to sure to understand this... The target dependency is for generating the file. Any suggestion / example of comment that you would like to see?

khwilliamson · 2025-07-07T03:13:47Z

Makefile.SH

@@ -1499,7 +1499,7 @@ cscope.out cscope: $(c) $(h)
 # The README below ensures that the dependency list is never empty and
 # that when MAKEDEPEND is empty $(FIRSTMAKEFILE) doesn't need rebuilding.

-MAKEDEPEND = Makefile makedepend_file makedepend
+MAKEDEPEND = Makefile makedependfile makedepend


I would add a comment like

The purpose of makedependfile is to guarantee that things get properly rebuilt when makedependfile.SH is changed

I have added the following comment to explain how $(FIRSTMAKEFILE) is generated and its relationship with makedepend and makedependfile. Hoping that is what you wanted...

# $(FIRSTMAKEFILE) is generated by the makedepend script. # The makedepend script uses the makedependfile script.

khwilliamson · 2025-07-07T03:14:54Z

.gitignore

@@ -27,7 +27,7 @@ bug*.pl
 /config.sh
 /makeaperl
 /makedepend
-/makedepend_file


The first few characters of the commit message are wrong

I made minor edits in the p.r. description for better English grammar. I realized it was a lot easier to just do them than to point them out.

This now looks good to me, but someone who is better at Makefiles should check

The first few characters of the commit message are wrong

What is wrong in the commit message? Do you mean instead something wrong in the p.r. description that you have fixed?

…hanges the content of makedependfile.SH Before the pull request: touch makedepend_file.SH && make => Nothing happens After the pull request: touch makedependfile.SH && make => makedependfile is regenerated => *.depends are regenerated => makefile is regenerated with dependencies Adding makedependfile.SH in variable SH of Makefile.SH creates the target makedependfile in generated makefile: diff before/perl5/Makefile after/perl5/Makefile 785a786,788 > makedependfile: makedependfile.SH config.sh > $(SHELL) makedependfile.SH > It has also the consequence that makedependfile is deleted with the rest of generated files: rm -rf $(addedbyconf)

jkeenan requested review from Corion and nwc10 July 4, 2025 14:36

dlaugt force-pushed the makedepend.file-target branch from 7dc43cf to 58cedfa Compare July 6, 2025 16:43

khwilliamson force-pushed the makedepend.file-target branch from 58cedfa to 3e4b685 Compare July 6, 2025 17:10

khwilliamson requested changes Jul 6, 2025

View reviewed changes

dlaugt changed the title ~~Add makedepend.file target in the generated Makefile~~ Update the makefile to regenerate makedependfile when the developer changes the content of makedependfile.SH Jul 6, 2025

dlaugt requested a review from khwilliamson July 6, 2025 20:39

khwilliamson reviewed Jul 7, 2025

View reviewed changes

dlaugt force-pushed the makedepend.file-target branch from 3e4b685 to 7eff038 Compare July 7, 2025 09:18

dlaugt requested a review from khwilliamson July 7, 2025 09:32

dlaugt force-pushed the makedepend.file-target branch from 7eff038 to c7cc452 Compare July 7, 2025 09:35

Update the makefile to regenerate makedependfile when the developer changes the content of makedependfile.SH #23402

Are you sure you want to change the base?

Update the makefile to regenerate makedependfile when the developer changes the content of makedependfile.SH #23402

Conversation

dlaugt commented Jul 4, 2025 • edited by khwilliamson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richardleach commented Jul 4, 2025

Uh oh!

dlaugt commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richardleach commented Jul 4, 2025

Uh oh!

dlaugt commented Jul 4, 2025

Uh oh!

richardleach commented Jul 4, 2025

Uh oh!

dlaugt commented Jul 4, 2025

Uh oh!

Leont commented Jul 4, 2025

Uh oh!

dlaugt commented Jul 5, 2025

Uh oh!

jkeenan commented Jul 5, 2025

Uh oh!

Leont commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dlaugt commented Jul 6, 2025

Uh oh!

dlaugt commented Jul 6, 2025

Uh oh!

khwilliamson left a comment

Choose a reason for hiding this comment

Uh oh!

dlaugt commented Jul 6, 2025

Uh oh!

khwilliamson Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

dlaugt Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

khwilliamson Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

khwilliamson Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlaugt Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dlaugt commented Jul 4, 2025 •

edited by khwilliamson

Loading

dlaugt commented Jul 4, 2025 •

edited

Loading

Leont commented Jul 6, 2025 •

edited

Loading

khwilliamson Jul 7, 2025 •

edited

Loading